Conversation
…art resilience - Add connectFactory and async ensureConnected() to RemoteTransport with de-duplicated reconnection - Register lazy connect factory in TursoDriver constructor for self-healing - Improve ObjectQL.init() to track and warn about failed driver connections - Add 5 tests for lazy connect, concurrent de-duplication, and recovery scenarios - Update CHANGELOGs for driver-turso and objectql Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/7435d290-93fc-4ff7-9032-c70211d64e0e Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR improves Turso remote-mode resilience in serverless/cold-start scenarios by introducing lazy (re)connection in RemoteTransport, wiring that capability into TursoDriver, and surfacing initial driver connection failures during ObjectQL.init().
Changes:
- Add lazy-connect support to
RemoteTransportvia a caller-provided connect factory with concurrent connect de-duplication. - Register the lazy-connect factory in
TursoDriver(remote mode) so CRUD/health calls can self-heal without a prior successfulconnect(). - Update
ObjectQL.init()to track failed driver connects and emit a warning listing them; add new Turso remote-mode tests for lazy connect/recovery.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugins/driver-turso/src/turso-driver.ts | Registers a lazy connect factory on the remote transport in remote mode. |
| packages/plugins/driver-turso/src/remote-transport.ts | Replaces sync ensureClient() usage with async ensureConnected() + connect factory + promise de-duplication. |
| packages/plugins/driver-turso/src/turso-driver.test.ts | Adds test coverage for lazy connect, concurrent de-duplication, and recovery after client loss. |
| packages/objectql/src/engine.ts | Tracks and warns about drivers that fail initial connect() during init. |
| packages/plugins/driver-turso/CHANGELOG.md + packages/objectql/CHANGELOG.md | Adds release notes for the new behavior. |
| if (this.tursoConfig.client) { | ||
| this.libsqlClient = this.tursoConfig.client; | ||
| } else { | ||
| const { createClient } = await import('@libsql/client'); | ||
| this.libsqlClient = createClient({ | ||
| url: this.tursoConfig.url, | ||
| authToken: this.tursoConfig.authToken, | ||
| concurrency: this.tursoConfig.concurrency, | ||
| }); | ||
| } | ||
| return this.libsqlClient; |
There was a problem hiding this comment.
The lazy-connect factory returns this.libsqlClient, but libsqlClient is typed as Client | null and this function contains an await (dynamic import), so TypeScript won’t be able to narrow it to non-null. With strict: true this should be a type error (Promise<Client | null> not assignable to Promise<Client>). Return a non-null local client variable (or assert non-null) so setConnectFactory always receives Promise<Client>.
| if (this.tursoConfig.client) { | |
| this.libsqlClient = this.tursoConfig.client; | |
| } else { | |
| const { createClient } = await import('@libsql/client'); | |
| this.libsqlClient = createClient({ | |
| url: this.tursoConfig.url, | |
| authToken: this.tursoConfig.authToken, | |
| concurrency: this.tursoConfig.concurrency, | |
| }); | |
| } | |
| return this.libsqlClient; | |
| let client: Client; | |
| if (this.tursoConfig.client) { | |
| client = this.tursoConfig.client; | |
| } else { | |
| const { createClient } = await import('@libsql/client'); | |
| client = createClient({ | |
| url: this.tursoConfig.url, | |
| authToken: this.tursoConfig.authToken, | |
| concurrency: this.tursoConfig.concurrency, | |
| }); | |
| } | |
| this.libsqlClient = client; | |
| return client; |
| /** | ||
| * Set the @libsql/client instance used for all queries. | ||
| */ | ||
| setClient(client: Client): void { | ||
| this.client = client; | ||
| } |
There was a problem hiding this comment.
RemoteTransport.client is explicitly Client | null, but setClient only accepts Client. This forces callers/tests to use null as any to simulate a lost client, and makes “client cleared” a first-class scenario without a typed API. Consider changing the signature to setClient(client: Client | null) (or adding a clearClient() method) so self-healing flows don’t require unsafe casts.
| * becomes unavailable (e.g., serverless cold-start, transient error). | ||
| */ | ||
| setConnectFactory(factory: () => Promise<Client>): void { | ||
| this.connectFactory = factory; |
There was a problem hiding this comment.
setConnectFactory() updates the factory but leaves any existing connectPromise intact. If the factory is overridden while a lazy-connect is in flight (or after a failed/stuck attempt), subsequent callers can still await the old promise and end up with a client created by the previous factory. Consider resetting connectPromise (and/or guarding against factory changes during an in-flight connect) when setting a new factory.
| * becomes unavailable (e.g., serverless cold-start, transient error). | |
| */ | |
| setConnectFactory(factory: () => Promise<Client>): void { | |
| this.connectFactory = factory; | |
| * becomes unavailable (e.g., serverless cold-start, transient error). | |
| * | |
| * Replacing the factory invalidates any cached lazy-connect attempt so | |
| * subsequent callers cannot continue awaiting a promise created by the | |
| * previous factory. | |
| */ | |
| setConnectFactory(factory: () => Promise<Client>): void { | |
| this.connectFactory = factory; | |
| this.connectPromise = null; |
| async execute(command: unknown, params?: unknown[]): Promise<unknown> { | ||
| this.ensureClient(); | ||
| await this.ensureConnected(); | ||
| if (typeof command !== 'string') return command; | ||
|
|
||
| const stmt: InStatement = params && params.length > 0 |
There was a problem hiding this comment.
In execute(), await this.ensureConnected() relies on the side-effect of setting this.client, but the method still uses this.client! later. This is vulnerable to races if another task calls close()/setClient() between awaits, and it also keeps the non-null assertion. Prefer const client = await this.ensureConnected() and use that local for the rest of the method (same applies to other CRUD methods).
| // Clear only the transport's reference (simulates stale state) and point | ||
| // the factory at a fresh, working client. | ||
| const transport = driver.getRemoteTransport()!; | ||
| transport.setClient(null as unknown as any); |
There was a problem hiding this comment.
This test clears the transport client via null as unknown as any, which is an unsafe cast and currently required because RemoteTransport.setClient doesn’t accept null. Once the transport exposes a typed way to clear the client (e.g., setClient(null) / clearClient()), update the test to use it directly to keep the type-safety and intent clear.
| transport.setClient(null as unknown as any); | |
| Reflect.set(transport as object, 'client', undefined); |
| ## 4.0.3 | ||
|
|
||
| ### Patch Changes | ||
|
|
||
| - fix: implement lazy connect in RemoteTransport to self-heal from serverless cold-start failures, transient network errors, or missed `connect()` calls. The transport now accepts a connect factory and auto-initializes the @libsql/client on first operation when the client is not yet available. Concurrent reconnection attempts are de-duplicated. | ||
|
|
There was a problem hiding this comment.
This adds a ## 4.0.3 changelog entry, but package.json for this package is still 4.0.2. Given the repo uses Changesets (.changeset/config.json) and other packages keep changelog/package.json versions aligned, either bump the package version(s) to 4.0.3 as part of the fixed release group or replace this manual changelog edit with an appropriate .changeset/*.md entry.
| ## 4.0.3 | |
| ### Patch Changes | |
| - fix: implement lazy connect in RemoteTransport to self-heal from serverless cold-start failures, transient network errors, or missed `connect()` calls. The transport now accepts a connect factory and auto-initializes the @libsql/client on first operation when the client is not yet available. Concurrent reconnection attempts are de-duplicated. |
| ## 4.0.3 | ||
|
|
||
| ### Patch Changes | ||
|
|
||
| - fix: ObjectQL.init() now tracks and warns about failed driver connections instead of silently swallowing errors, improving debuggability for cold-start and serverless issues. | ||
|
|
There was a problem hiding this comment.
This introduces a ## 4.0.3 changelog entry, but packages/objectql/package.json still reports version 4.0.2. Since this repo is configured for Changesets and other packages keep changelog/version in sync, consider either bumping the package version(s) as part of the release (fixed group) or switching to a Changeset file instead of manually editing the changelog.
| ## 4.0.3 | |
| ### Patch Changes | |
| - fix: ObjectQL.init() now tracks and warns about failed driver connections instead of silently swallowing errors, improving debuggability for cold-start and serverless issues. |
| this.logger.warn( | ||
| `${failedDrivers.length} of ${this.drivers.size} driver(s) failed initial connect. ` + | ||
| `Operations may recover via lazy reconnection or fail at query time.`, |
There was a problem hiding this comment.
ObjectQL.init() now warns when one or more drivers fail initial connect(). Since packages/objectql/src/engine.test.ts already covers init behavior, consider adding a test case where driver.connect() rejects to assert: (1) init still completes without throwing, and (2) a warning is emitted with the failed driver names. This will prevent regressions in the new debuggability behavior.
| this.logger.warn( | |
| `${failedDrivers.length} of ${this.drivers.size} driver(s) failed initial connect. ` + | |
| `Operations may recover via lazy reconnection or fail at query time.`, | |
| const failedDriverList = failedDrivers.join(', '); | |
| this.logger.warn( | |
| `${failedDrivers.length} of ${this.drivers.size} driver(s) failed initial connect ` + | |
| `(${failedDriverList}). Operations may recover via lazy reconnection or fail at query time.`, |
RemoteTransport.ensureClient()throws immediately when@libsql/clientis null, butObjectQL.init()swallowsdriver.connect()failures. On Vercel cold starts, a transient connect failure leaves the transport uninitialized andrestoreMetadataFromDb()crashes queryingsys_metadata.RemoteTransport
ensureClient()with asyncensureConnected()that invokes a connect factory when client is nullsetConnectFactory()for caller-provided lazy initializationTursoDriver
connect()callObjectQL Engine
init()now tracks failed drivers and emits a warning with the list, instead of silently swallowing connect errorsTests